-
-
Notifications
You must be signed in to change notification settings - Fork 104
Do not enable attestations by default #370
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not enable attestations by default #370
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It hasn't been experimental for almost a year. There was a small period of opt-in when things were being sorted out, that's it. There's no random breakage. It was related to an unforeseen change in a dependency that's been sorted quickly. I'm not going to accept this breaking change, sorry.
@woodruffw we should probably drop that experimental note from the README. Looks like it's long overdue... |
i don't understand how it would be considered stable when the failures as described in #365 are still occurring and from what i gather from skimming over the following discussion nobody really knows the cause(s). |
@funkyfuture that was resolved in a day or so. I just haven't had a chance to close that issue, because I wanted to track a few follow-up discussions. The bug is known. It was in PyPI's dependency and that's addressed. There was also a standardization issue that's documented in PyPUG now. There haven't been any other reports. I don't know where you're getting those imaginary breakages that are "still occurring" — I haven't seen a single proof of that. You could as well argue that the entire Trusted Publishing thing should be reverted because there were bugs along the road and I would say that nothing is stopping you from going back to using API tokens 🤷♂️ and keeping your uploads less transparent. That said, there's no merrit in disabling attestations. This will remain secure by default. But we should update README. Such a PR would be very welcome indeed. |
Yeah, the feature is de facto no longer experimental (and de sure, in the sense that it's tied to a fully accepted PEP). I'll own updating the documentation to clarify that. |
Done with #371; the PyPI docs also still say "experimental," so I'll send a PR to update those later today. |
i encountered it yesterday. |
That's a different error; the original issue is specifically about a filename matching/normalization bug, not about any and all errors that can happen while verifying an attestation. In your case, it looks like you have your Trusted Publisher set up against |
Oh, that's because reusable workflows aren't officially supported, pending a fix in PyPI. It works for one obscure corner case for some people by accident. But a bugfix in Warehouse (PyPI) is required and it won't be backwards compatible with hacks that made it possible for said corner case. This isn't really an attestations thing but Trusted Publishing as a whole (the OIDC layer to be precise). We've spent hours at the PyCon US sprints in May strategizing the migration and it's recorded somewhere in a Warehouse issue. I believe Facundo is still waiting for funding to complete that work. TL;DR is that your use is unsupported, but the supported uses are functional and stable. |
Ah yeah, I think this is a (new) edge case where the reusable workflow handling is rough: the attestation is from |
@woodruffw I'm pretty sure I saw this case earlier. I was hoping you PR #306 with hints would steer people in the right direction.. |
Okay, that gives me more incentive to revive that PR. I'll try and get to it this week 😅 |
thank you for your hints and elaborations. from a user perspective i'd like to leave some remarks:
|
No problem, thanks for providing more details.
It's mentioned here: https://docs.pypi.org/trusted-publishers/troubleshooting/#reusable-workflows-on-github I suppose we could probably link to the troubleshooting page in more places; it probably makes sense to have this action include it as part of the diagnostic output.
I agree, although your intuition here unfortunately doesn't match GitHub's security model: a reusable workflow can be reused by anybody including forks and arbitrary repos, so it can never be used to uniquely identify a CI/CD run for something like Trusted Publishing. So PyPI can never rely solely on the reusable workflow identity; it must always use the caller identity in some form, since that's grounded against the repository. That's something we could definitely improve the documentation for however, since GitHub doesn't (IMO) do a great job of explaining that. |
This proposes to revert the default to create attestations.
The documentation clearly states that it is an experimental
feature, while #365 demonstrates that it isn't a fully
evolved functionality, and it randomly breaks publishing
pipelines.